stop installing incompatible recursive extension dependencies #10716
Conversation
|
E2E Tests 🚀 |
| } | ||
| // --- End Positron --- | ||
| const manifest = await this.extensionGalleryService.getManifest(gallery, CancellationToken.None); | ||
| if (!manifest) { |
There was a problem hiding this comment.
This strange formatting must have been left over from an upstream merge or something.
| 'vscode.r', | ||
| 'jeanp413.open-remote-ssh', | ||
| 'ms-python.python', | ||
| 'ms-python.vscode-python-envs', |
There was a problem hiding this comment.
This new code won't delete these extensions if you already had them. Should we go that far? (making sure we don't delete builtin ones like our version of ms-python.python of course)
There was a problem hiding this comment.
I still think deleting them is very aggressive, since we can't know if we installed anything here or if the user did. I think the best option is a notification like in #10712.
juliasilge
left a comment
juliasilge
left a comment
There was a problem hiding this comment.
This is working for me. If I totally blow away all state and extensions I have ever installed previously, I do not see the problematic extension installed and I only see the ones on our list:
I see the expected error if I try to install ms-python.vscode-python-envs directly.
I am not super familiar with this part of the code base, but the changes look reasonable to me! 🚀
| 'jeanp413.open-remote-ssh', | ||
| 'ms-python.python', | ||
| 'ms-python.vscode-python-envs', | ||
| 'GitHub.copilot-chat' |
There was a problem hiding this comment.
| 'GitHub.copilot-chat' | |
| 'github.copilot-chat' |
isPositronExtensionCompatible converts extension IDs to lowercase before comparison, maybe we should include in the function comment that these need to be all lower case?
There was a problem hiding this comment.
great catch!!
There was a problem hiding this comment.
cool, fixed, interestingly this bug was present before too!
Addresses #10423. Adds the
ms-python.vscode-python-envsextension to the list of incompatible Positron extensions.I also noticed that it was still getting installed as a recursive dependency of other extensions like Pyrefly. So this PR also adds some code that checks for compatibility of recursive dependencies before installing them. In order to do that, I had to move some stuff from the workbench level to the platform level.
Release Notes
New Features
Bug Fixes
QA Notes
The python environments extension shouldn't be installable directly or as a dependency of e.g. pyrefly or ty.
If you had it before, it won't delete it.
@ Julia: add a release note reminding users to uninstall this extension.